Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Envman functions #819

Merged
merged 12 commits into from
Oct 10, 2022
Merged

Envman functions #819

merged 12 commits into from
Oct 10, 2022

Conversation

godrei
Copy link
Contributor

@godrei godrei commented Oct 6, 2022

Checklist

Version

Requires a MINOR version update

Context

This PR makes Bitrise CLI use envman functionalities through exported functions rather than through envman binary calls.

Changes

  • Update tools.Envman<Function>s to use envman exported functions.

Investigation details

Decisions

@godrei godrei mentioned this pull request Oct 6, 2022
2 tasks
cmd.SetTimeout(timeout)
cmd.SetHangTimeout(noOutputTimeout)
cmd.SetStandardIO(inReader, outWriter, errWriter)
cmd.AppendEnv("PWD=" + workDirPth)
cmd.SetEnv(append(envs, "PWD="+workDirPth))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this better than the existing AppendEnv() call?

Copy link
Contributor Author

@godrei godrei Oct 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppendEnv is working with a single environment variable + checking if the command has Env set or not is unnecessary because it will always be nil, and the envs we set already contain the os.Environ.

Although we don't need both functions, so I removed cmd.AppendEnv in 73afe9d

@godrei godrei marked this pull request as ready for review October 10, 2022 09:57
cli/run_util.go Outdated Show resolved Hide resolved
tools/tools.go Show resolved Hide resolved
@godrei godrei merged commit e4ef36e into master Oct 10, 2022
@godrei godrei deleted the envman-functions branch October 10, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants